feat: compatible codeSplitting config with umi #1669
Conversation
xusd320
requested review from
sorrycc,
stormslowly,
PeachScript and
Jinbao1001
Walkthrough该拉取请求对代码拆分和优化过程进行了重要修改，主要集中在 Changes
Possibly related PRs
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
xusd320
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (1)
crates/mako/src/generate/optimize_chunk.rs (1)
Line range hint 1-1000: 代码结构建议优化
代码整体实现较为复杂，建议进行以下优化：
-
文档完善：
- 建议为主要的优化策略添加详细的文档注释，说明其使用场景和优化目标
- 关键算法（如 chunk 大小优化）需要添加实现原理说明
-
错误处理优化：
panic!("Duplicated chunk: {}", &info_chunk_id.id)建议改为返回Result- 建议添加更多的错误场景处理
-
代码重构建议：
optimize_chunk_size方法过长，建议拆分为多个子函数- 可以考虑将优化策略抽象为独立的 trait
建议将 optimize_chunk_size 重构如下：
- fn optimize_chunk_size(&self, optimize_chunks_infos: &mut Vec<OptimizeChunksInfo>) {
+ fn optimize_chunk_size(&self, optimize_chunks_infos: &mut Vec<OptimizeChunksInfo>) -> Result<(), ChunkError> {
+ self.calculate_chunk_sizes(optimize_chunks_infos)?;
+ self.apply_size_constraints(optimize_chunks_infos)?;
+ self.split_oversized_chunks(optimize_chunks_infos)?;
+ Ok(())
+ }📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (3)
- crates/mako/src/generate/optimize_chunk.rs (1 hunks)
- e2e/fixtures/code-splitting.granular/expect.js (1 hunks)
- packages/bundler-mako/index.js (2 hunks)
🔇 Additional comments (2)
e2e/fixtures/code-splitting.granular/expect.js (1)
13-20: 文件命名模式的更改符合预期
测试用例中的文件命名模式已更新，移除了数字前缀（如 lib_0_），使其更加简洁。这个改动与 Umi 的代码分割配置保持一致，同时保持了原有的功能验证逻辑不变。
运行以下脚本验证新的文件命名约定是否在其他测试用例中保持一致：
✅ Verification successful
文件命名模式更改已验证完成
通过检查代码库，我们发现：
- 新的命名模式（
lib_lib1-async.js、lib_lib2-async.js、lib_shared1-async.js、lib_shared2-async.js）仅在e2e/fixtures/code-splitting.granular/expect.js文件中使用 - 这些更改是独立且局部的，不会影响其他测试用例
- 测试断言的功能逻辑保持不变，仅调整了文件命名约定
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# 描述：检查其他测试用例中的文件命名模式
# 预期：新的命名模式应该在相关测试中保持一致
# 搜索所有包含旧命名模式的测试文件
rg -l "lib_[0-9]+_.*-async\.js" "e2e/fixtures"
Length of output: 46
Script:
#!/bin/bash
# 搜索所有包含 lib_lib 或 lib_shared 模式的测试文件
rg -l "lib_(lib|shared)[0-9]+-async\.js" "e2e/fixtures"
# 检查代码分割相关的测试文件内容
fd "expect.js" "e2e/fixtures" -t f -E "node_modules" --exec rg -l "lib_.*-async\.js" {}
Length of output: 235
Script:
#!/bin/bash
# 检查代码分割测试文件的完整内容，以了解命名模式的上下文
cat e2e/fixtures/code-splitting.granular/expect.js
# 检查相关的测试用例实现
fd -t f -e js "code-splitting.granular" "e2e/fixtures" --exec cat {}
Length of output: 1326
crates/mako/src/generate/optimize_chunk.rs (1)
49-51: 优化阶段顺序调整需要验证影响
优化阶段的执行顺序已调整，将 optimize_chunk_size 移至 optimize_name_suffix 之后。这个改动可能会影响最终的代码分割结果，建议验证以下几点：
- 分割后的代码块大小是否仍然符合预期
- 代码块的命名是否仍然保持一致性
- 是否会对热更新性能产生影响
✅ Verification successful
Let me gather more information about the implementation details of these optimization functions to better understand their dependencies and potential impacts.
优化阶段顺序调整是安全的
通过分析代码实现，可以确认这个顺序调整是安全的，原因如下：
optimize_name_suffix只负责根据策略（包名或依赖哈希）对chunk进行命名和分组，不会影响chunk的大小计算optimize_chunk_size主要处理chunk的大小优化，包括分割过大的chunk和移除过小的chunk，其实现不依赖于chunk的命名方式- 两个函数都是独立工作的，它们通过
optimize_chunks_infos共享数据，但对数据的修改是互不干扰的
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# 验证代码分割结果的一致性
# 1. 检查是否存在相关的测试用例
rg -l "optimize_chunk.*test"
# 2. 检查是否有其他依赖于优化顺序的代码
rg "optimize_(chunk_size|name_suffix)"
Length of output: 555
Script:
#!/bin/bash
# 检查optimize_chunk_size和optimize_name_suffix的具体实现
ast-grep --pattern 'fn optimize_chunk_size($$$) {
$$$
}'
ast-grep --pattern 'fn optimize_name_suffix($$$) {
$$$
}'
# 检查是否有其他文件引用这些方法
rg -A 3 "optimize_chunk_size|optimize_name_suffix" --type rust
Length of output: 21543
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1669 +/- ##
=========================================
Coverage ? 55.59%
=========================================
Files ? 173
Lines ? 17508
Branches ? 0
=========================================
Hits ? 9733
Misses ? 7775
Partials ? 0 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (3)
crates/mako/src/config/code_splitting.rs (1)
88-88: 建议为字段添加文档注释
为了提高代码的可维护性，建议添加文档注释来说明此字段的用途、单位和使用场景。
建议添加如下文档：
#[serde(default)]
+ /// 包大小的最小阈值（以字节为单位）
+ /// 当包的大小小于此阈值时，它将被合并到其他chunk中
pub min_package_size: Option<usize>,crates/mako/src/plugins/ssu.rs (2)
Line range hint 279-307: 建议优化缓存机制的实现
当前缓存实现在处理依赖关系时可以进行以下改进：
- 建议将魔法字符串提取为常量
- 可以增加错误重试机制
- 建议添加缓存过期策略
建议重构如下：
+ const CACHE_VERSION: &str = "1";
+ const MAX_RETRY_ATTEMPTS: i32 = 3;
+ const CACHE_TTL_DAYS: i64 = 7;
fn write_current_cache_state(&self, context: &Arc<Context>) -> Result<()> {
let cache_file = context.root.join("node_modules/.cache_mako/meta.json");
+ let cache = self.current_state.lock().unwrap();
+ let cache_data = CacheData {
+ version: CACHE_VERSION.to_string(),
+ created_at: chrono::Utc::now(),
+ data: (*cache).clone(),
+ };
- let cache = self.current_state.lock().unwrap();
- fs::write(cache_file, serde_json::to_string(&*cache).unwrap())?;
+ fs::write(cache_file, serde_json::to_string(&cache_data)?)?;
Ok(())
}Line range hint 309-319: 建议增强错误处理机制
当前的错误处理使用了 if let 语法，但可以更加健壮：
- 建议添加详细的错误信息
- 考虑区分不同类型的错误情况
建议修改如下：
fn load_cached_state(&self, context: &Arc<Context>) -> Option<CacheState> {
let cache_file = context.root.join("node_modules/.cache_mako/meta.json");
- if let Ok(content) = fs::read_to_string(cache_file)
- && let Ok(disk_cache) = serde_json::from_str(&content)
- {
- return Some(disk_cache);
- }
+ match fs::read_to_string(&cache_file) {
+ Ok(content) => match serde_json::from_str(&content) {
+ Ok(disk_cache) => Some(disk_cache),
+ Err(e) => {
+ debug!("缓存解析失败: {}", e);
+ None
+ }
+ },
+ Err(e) => {
+ debug!("读取缓存文件失败: {}", e);
+ None
+ }
+ }
- None
}📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (5)
- crates/mako/src/config/code_splitting.rs (2 hunks)
- crates/mako/src/generate/optimize_chunk.rs (4 hunks)
- crates/mako/src/plugins/ssu.rs (2 hunks)
- e2e/fixtures/code-splitting.granular/expect.js (1 hunks)
- packages/bundler-mako/index.js (2 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- crates/mako/src/generate/optimize_chunk.rs
- e2e/fixtures/code-splitting.granular/expect.js
🔇 Additional comments (3)
crates/mako/src/config/code_splitting.rs (1)
88-88: 字段重命名是一个破坏性更改
将 min_module_size 重命名为 min_package_size 使配置与 Umi 保持一致是个好的改动，但这是一个破坏性更改。建议在发布说明中标注此更改。
✅ Verification successful
字段重命名已正确实施且用法一致
代码库中所有使用该配置字段的地方都已更新为 min_package_size，包括默认配置、SSU 插件和代码分块优化逻辑。这个破坏性更改已经在整个代码库中得到了一致的实施。
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# 检查是否有其他文件仍在使用旧的字段名
rg "min_module_size"
Length of output: 258
Script:
#!/bin/bash
# 检查这些文件的内容以确定它们是否是测试文件或示例代码
cat e2e/fixtures/code-splitting.granular/node_modules/lib1/index.js
cat e2e/fixtures/code-splitting.granular/node_modules/lib2/index.js
# 检查是否有其他使用此配置的代码
rg -A 3 "CodeSplittingConfig"
Length of output: 380130
Script:
#!/bin/bash
# 检查是否有其他配置文件或测试文件使用此配置
rg -A 3 "min_module_size|min_package_size" --type rust
Length of output: 1917
crates/mako/src/plugins/ssu.rs (1)
174-174: 配置字段更新符合 umi 规范
将 min_module_size 更改为 min_package_size 使配置与 umi 的代码分割配置保持一致。这个改动反映了从模块级别到包级别的粒度转变。
执行以下脚本验证配置兼容性：
Also applies to: 185-185
✅ Verification successful
配置字段更新已完成且实现正确
代码库中的 min_module_size 已全面更新为 min_package_size，且更新涵盖了所有相关实现：
- 配置定义（
config/code_splitting.rs） - 插件实现（
plugins/ssu.rs） - 分块优化逻辑（
generate/optimize_chunk.rs）
e2e 测试文件中的注释提及 min_module_size 不影响功能，仅作为文档说明。
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# 描述：验证代码库中是否还存在 min_module_size 的使用
rg "min_module_size"
Length of output: 258
Script:
#!/bin/bash
# 检查代码分割相关的配置和测试用例
rg -A 5 "min_package_size|min_module_size" --type rust
# 检查测试文件中的相关配置
fd -e rs test -x rg -l "min_package_size|min_module_size" {}
Length of output: 2808
packages/bundler-mako/index.js (1)
629-629: 配置项使用正确
codeSplitting 配置项已正确集成到 makoConfig 对象中。
Compatible mako codeSplitting config for umi/codeSplitting
Summary by CodeRabbit